cli: Remove unreachable recovery and search paths - #105
BenWestgate wants to merge 2 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted release-gate re-review performed at the maintainer's request and disclosed per docs/developer/AI_POLICY.md.
ACK d886238.
This is the same two-commit cleanup previously reviewed as #98, replayed directly on refreshed #57. Stable patch-ids match #98 exactly. The changes only remove provably redundant correction-plan state and the already-type-checked share filter; correction scheduling, capture accounting, wallet behavior, and public input validation are unchanged.
Current-base verification: 101 focused CLI/correction tests passed, and the installed source is 5,179 logical lines under the authorized <5200 cap. No correctness blocker found.
BenWestgate
added
area: cli
a7efaae to
054e8d9
Compare
d886238 to
52c23b3
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Exact-head release-gate re-review: ACK 52c23b3. The two-commit cleanup removes only redundant correction-plan state and a share filter after the all-Share type check; no correction scheduling, capture accounting, wallet behavior, or public validation changes. On the full refreshed restore stack through #95, 935 tests pass normally and under python -O; Ruff, format, strict mypy, correction constants, all 57 frozen differential cases, and the production-size test pass. No code blocker found. Both commits are agent-authored and still require responsible-human rewrite/squash before integration.
054e8d9 to
115f2c2
Compare
Every correction plan returned its target set, that same set as primary, an empty reduced set, and a true timed flag. Only the targets and primary set were consumed. Derive primary from targets at the call site and remove the other fields. The search engine also accepted reduced without reading it, so remove that argument and update its test and benchmark callers. Search order and capture accounting remain unchanged. Refs #46.
The preceding all-isinstance check rejects every non-share, so the list-comprehension predicate in recovery could never discard an item. Pass the validated list directly, using a type cast to express the established invariant to mypy. Recovery still copies and validates the sequence internally. Refs #46.
52c23b3 to
361feb7
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head re-review: ACK 361feb7.
The current diff is still the narrow behavior-preserving cleanup: _correction_plan() returns only the target lengths it actually uses, _search_many() drops an unread reduced parameter, and recovery passes the already-validated all-Share list directly. No scheduling, capture accounting, wallet behavior, or public validation changes are introduced. There are no inline review threads, and exact-head Python-package run 660 succeeded.
No code blocker found. The two agent-authored commits still require responsible-human rewrite/squash under repository policy before integration.
This branch has not been deployed
1 participant
Restore the focused cleanup review boundary after the historical #98 cleanup was absorbed into later stack work.
The PR remains two small behavior-preserving cleanup commits:
_correction_planprimary/reduced/timed state and_search_many(reduced=...)argument;recover_secret()instead of filtering it a second time.No correction scheduling, capture accounting, wallet behavior, or public input validation changes.
Review shape
Current head
361feb7is stacked directly on current #57 (115f2c2). The second cleanup commit retains the exact reviewed stable patch-id from historical #98. The first cleanup overlaps the adjacent_correction_candidates(..., optional_only=...)context introduced by merged #42, so its stable patch-id changed on replay; the cleanup edits themselves remain the reviewed behavior-preserving reduction.The production-size gate remains the maintainer-authorized
<5200; current-head CI includes that enforcement.Validation
Current-head Python-package run 660 succeeded, with later duplicate current-head runs also green. The prior full validation of the same two cleanup commits passed the full suite normally and under
python -O, Ruff check/format, strict mypy, all 57 frozen differential correction cases, andgit diff --check.Codex re-reviewed current head
361feb7and found no code blocker; there are no inline review threads.Human review/integration order from here is #57 → #105 → #99 → refreshed #80 → #81 → #95. #99 is already stacked on this exact head. After #99 integrates, refresh #80 once onto that settled Core/CLI tip before carrying #81/#95 forward.
Refs #46 and #98. These cleanup commits remain agent-authored and require responsible-human rewrite or squash under repository policy before final integration.